Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Format files using isort and black #1166

Merged
merged 1 commit into from
Jun 8, 2021
Merged

Conversation

BreadGenie
Copy link
Contributor

The cvedb.py file has isort issues and made all CI checks fail and also while working on a PR I ran black on the entire directory and found that black formatted 19 files (had to restore files one by one since my work was uncommited).

@codecov-commenter
Copy link

Codecov Report

Merging #1166 (046b2a6) into main (66e1df3) will increase coverage by 2.15%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1166      +/-   ##
==========================================
+ Coverage   79.98%   82.14%   +2.15%     
==========================================
  Files         196      196              
  Lines        3383     3383              
  Branches      381      381              
==========================================
+ Hits         2706     2779      +73     
+ Misses        576      512      -64     
+ Partials      101       92       -9     
Flag Coverage Δ
longtests 82.14% <ø> (+2.15%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
cve_bin_tool/checkers/sqlite.py 58.82% <ø> (ø)
cve_bin_tool/cvedb.py 86.47% <ø> (+0.71%) ⬆️
cve_bin_tool/error_handler.py 87.93% <ø> (ø)
cve_bin_tool/extractor.py 63.28% <ø> (+3.12%) ⬆️
cve_bin_tool/file.py 95.45% <ø> (ø)
cve_bin_tool/output_engine/console.py 100.00% <ø> (ø)
cve_bin_tool/version_scanner.py 83.89% <ø> (+1.69%) ⬆️
cve_bin_tool/version_signature.py 76.59% <ø> (ø)
test/test_checkers.py 98.07% <ø> (ø)
test/test_cli.py 93.10% <ø> (+11.82%) ⬆️
... and 18 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 66e1df3...046b2a6. Read the comment docs.

@Molkree
Copy link
Contributor

Molkree commented Jun 5, 2021

Just for the context, isort check fails now because #1117 got merged, it had all green checks as they were run before isort check was fixed in #1146.

As for the Black, your run on all directory resulting in some changes is, I bet, due to the fact that you are using some recent version of Black and not the one in .pre-commit-config.yaml. In pre-commit it is Black 20.8b1 as well as in CI. As we can see in this case these changes are backwards compatible, cause CI using an older version has passed. But ultimately you should just use pre-commit (pre-commit run -a).

As I've mentioned in #970, switching to pre-commit.ci seems like the best idea.

But as you've run some newer Black on the codebase in this PR, might as well update the version in pre-commit config. And then we could just run this exact version in CI using pre-commit instead of being tied to the 3rd party's Black check version (jpetrucciani/black-check, wasn't updated since September). This is how you add CI check using pre-commit manually (example), until this repo maybe decides to switch to pre-commit.ci.

Might update isort version too.

@BreadGenie
Copy link
Contributor Author

BreadGenie commented Jun 6, 2021

Thanks! I'll take a look at it once I get back to my machine (would take a couple days).

Copy link
Contributor

@terriko terriko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, thanks for getting this fixed!

@terriko terriko merged commit 7b95f5a into intel:main Jun 8, 2021
@BreadGenie BreadGenie deleted the isort-black branch June 8, 2021 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants